Conversation
rajanyadav0307
force-pushed
the
Issue-3613
branch
from
477c68d to
53aeaa9
Compare
|
@sbaluja , Thanks |
|
@sbaluja , Please let me know if we are expecting more for the PR. Thanks |
sbiscigl
left a comment
sbiscigl
left a comment
There was a problem hiding this comment.
Left a review on how to handle and where to put this variable properly, its a bit different than what you were doing before, if you want to update it to that would be more than happy to accept it
cmake/compiler_settings.cmake
Outdated
| # which can override a consumer's chosen language level. Keep default behavior for | ||
| # backward compatibility, but allow packagers/consumers to disable exporting the C++ | ||
| # standard (and related policy flags) via pkg-config. | ||
| option(NO_CPPSTD_IN_PKG_CONFIG |
There was a problem hiding this comment.
This option is in the wrong place, this option should appear and act identically to ENABLE_RTTI which is another compiler setting turned on and off.
this is found in the top level CMakeLists.txt
option(ENABLE_RTTI "Flag to enable/disable rtti within the library" ON)in that list of options we should include one for this case
option(NO_CPPSTD_IN_PKG_CONFIG "Removes cpp language standard from being included in pkg config installs" OFF)then this can be used more easily where the actual compile flags are being set for gcc and this can look exactly like RTTI i.e.
macro(set_gcc_flags)
list(APPEND AWS_COMPILER_FLAGS "-fno-exceptions")
if(NOT NO_CPPSTD_IN_PKG_CONFIG)
list(APPEND AWS_COMPILER_FLAGS "-std=c++${CPP_STANDARD}")
endif()
if(NOT ENABLE_RTTI)
list(APPEND AWS_COMPILER_FLAGS "-fno-rtti")
endif()
# other code ...
endmacro()
cmake/compiler_settings.cmake
Outdated
| target_compile_options(${target} PRIVATE "${AWS_COMPILER_FLAGS}") | ||
| string(REPLACE ";" " " _TMP "${AWS_COMPILER_FLAGS}") | ||
| set(PKG_CONFIG_CFLAGS "${_TMP}" CACHE INTERNAL "C++ compiler flags which affect the ABI") | ||
| if(NO_CPPSTD_IN_PKG_CONFIG) |
There was a problem hiding this comment.
please refer to the other comment, thats likely where and how we want to handle this option
Sign-off: Rajan Y.(rajanyadav0307@gmail.com)
rajanyadav0307
force-pushed
the
Issue-3613
branch
from
7035938 to
318c7a1
Compare
|
created #3792 to run CI on and merge |
|
merged, sorry for the long wait and thanks for contributing! |
Thank you @sbiscigl for merging the changes. I really appreciate it. Looking forward for contributing more. |
3 participants
Issue #, if available:
#3613
Description of changes:
This PR adds an opt-out CMake option to prevent the AWS SDK for C++ from exporting
the C++ language standard via pkg-config
Cflags.Currently, the generated
.pcfiles include compiler policy flags (e.g.-std=c++<N>,-fno-exceptions) derived fromAWS_COMPILER_FLAGS. Downstreamprojects that consume the SDK using pkg-config (for example, Meson or custom
build systems) blindly apply these flags, which can override the consumer’s
chosen C++ standard and cause build failures.
To address this without breaking existing users, this PR introduces a new option:
When enabled, the generated pkg-config
Cflagswill no longer include:-std=c++<N>(primary issue reported)-fno-exceptions(also identified as leaking semantics into downstream builds)The default behavior remains unchanged to preserve backward compatibility.
CMake-based consumers are unaffected, as these flags remain
PRIVATEto SDKtargets.
Check all that applies:
Did a review by yourself.
Added proper tests to cover this PR.
Two Docker-based tests were used:
- One Dockerfile builds the upstream SDK and a minimal downstream project
to reliably reproduce the reported failure. Docker_Issue_Repro.zip
- A second Dockerfile builds the SDK with this patch enabled and verifies
that the downstream project builds successfully.Docker-Fix-Verify.zip
Checked if this PR is a breaking (APIs have been changed) change.
Checked if this PR will not introduce cross-platform inconsistent behavior.
Checked if this PR would require a ReadMe/Wiki update.
Check which platforms you have built SDK on to verify the correctness of this PR.
Screenshots:
Issue Reproduced:
Fix-Verify:
By submitting this pull request, I confirm that you can use, modify, copy, and
redistribute this contribution, under the terms of your choice.
Sign-off: Rajan Y.(rajanyadav0307@gmail.com)